New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

layout_moyo-header solution #5830

Open

ibabiuk wants to merge 3 commits into mate-academy:master from ibabiuk:develop

ibabiuk commented Dec 17, 2024

DEMO LINK
TEST REPORT LINK
Header height is set in 1 place (for the links)
Content is vertically centered (for any header height)
CSS is used to show all letters in Uppercase (don't type them in HTML)
Logo is an image wrapped with a link
CSS Variable is used for a blue color
Pseudo-element is used for a blue line below the active link
Code follows all the Code Style Rules ❗️

ibabiuk added 2 commits

December 17, 2024 15:51


          layout_moyo-header solution

8a32d03


          layout_moyo-header solution

f7a016e

mateacademy-ai-mentor suggested changes

View reviewed changes

mateacademy-ai-mentor left a comment

Thank you for your hard work on this project! 😊 While you've made a great effort, there are a few critical issues that need to be addressed to meet the core requirements of the task. Specifically, the use of the universal selector in CSS is against the checklist requirements due to potential performance impacts. Additionally, the task requires the use of a CSS variable for the blue color, which is currently missing. These issues are important to resolve to ensure your code aligns with the task specifications. Please make these changes, and feel free to reach out if you need any help! Keep up the great work! 🚀

✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨

🔄 Fix the issues, commit the changes, and then re-request my review using the Re-request review button in the Reviewers tab.

src/index.html Outdated

                 </head>
                 <body>
                   <h1>Moyo header</h1>
+                  <header class="header">
+                    <a
+                      href="#"

mateacademy-ai-mentor Dec 17, 2024

Using href="#" is not recommended as it can cause unexpected behavior when clicked. Consider using a valid URL or javascript:void(0); if the link is meant to be a placeholder.

src/index.html Outdated

+                      <ul class="navigation">
+                        <li>
+                          <a
+                            href="#"

mateacademy-ai-mentor Dec 17, 2024

Using href="#" is not recommended as it can cause unexpected behavior when clicked. Consider using a valid URL or javascript:void(0); if the link is meant to be a placeholder.

src/index.html Outdated

+                        </li>
+                        <li>
+                          <a
+                            href="#"

mateacademy-ai-mentor Dec 17, 2024

Using href="#" is not recommended as it can cause unexpected behavior when clicked. Consider using a valid URL or javascript:void(0); if the link is meant to be a placeholder.

src/index.html Outdated

+                        </li>
+                        <li>
+                          <a
+                            href="#"

mateacademy-ai-mentor Dec 17, 2024

Using href="#" is not recommended as it can cause unexpected behavior when clicked. Consider using a valid URL or javascript:void(0); if the link is meant to be a placeholder.

src/index.html Outdated

+                        </li>
+                        <li>
+                          <a
+                            href="#"

mateacademy-ai-mentor Dec 17, 2024

Using href="#" is not recommended as it can cause unexpected behavior when clicked. Consider using a valid URL or javascript:void(0); if the link is meant to be a placeholder.

src/index.html Outdated

+                        </li>
+                        <li>
+                          <a
+                            href="#"

mateacademy-ai-mentor Dec 17, 2024

Using href="#" is not recommended as it can cause unexpected behavior when clicked. Consider using a valid URL or javascript:void(0); if the link is meant to be a placeholder.

src/index.html Outdated

+                        </li>
+                        <li>
+                          <a
+                            href="#"

mateacademy-ai-mentor Dec 17, 2024

Using href="#" is not recommended as it can cause unexpected behavior when clicked. Consider using a valid URL or javascript:void(0); if the link is meant to be a placeholder.

src/index.html Outdated

+                        </li>
+                        <li>
+                          <a
+                            href="#"

mateacademy-ai-mentor Dec 17, 2024

Using href="#" is not recommended as it can cause unexpected behavior when clicked. Consider using a valid URL or javascript:void(0); if the link is meant to be a placeholder.

src/style.css

+                margin: 0;
+                padding: 0;
+                box-sizing: border-box;
+                font-family: Roboto, sans-serif;

mateacademy-ai-mentor Dec 17, 2024

The font-family is set globally, which is fine, but ensure that the 'Roboto' font is correctly loaded in your HTML file. This is already done in your HTML file using Google Fonts, so this line is correctly implemented.

src/style.css Outdated Show resolved Hide resolved


          moyo-header fix

25317d0

ibabiuk requested a review from mateacademy-ai-mentor

December 17, 2024 14:54

mateacademy-ai-mentor approved these changes

View reviewed changes

mateacademy-ai-mentor left a comment

Review Summary

Great job on your submission! 🎉 Your HTML file is well-structured and follows standard practices, with appropriate use of Google Fonts and local images. However, there are a couple of issues in the CSS file that need attention:

Universal Selector Usage: The CSS file uses the universal selector * to reset margins and paddings. This approach can lead to performance issues and is against the checklist requirements. Consider applying styles more selectively to improve performance.
CSS Variable for Blue Color: The task specifies using a CSS variable for the blue color. Currently, the color #00acdc is hardcoded. Defining a CSS variable for this color and using it throughout the stylesheet will enhance maintainability and align with the task requirements.

Despite these issues, the decision is to APPROVE your pull request to help you move forward with your learning. Please review the comments and make the necessary improvements to your code. Addressing these issues will not only meet the task specifications but also enhance the overall quality and performance of your code. Keep up the good work and continue learning from these experiences! 🚀

✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨

src/style.css

		@@ -1,3 +1,71 @@
		* {

mateacademy-ai-mentor Dec 17, 2024

Using the universal selector (*) is against the checklist requirements due to potential performance impacts. Consider applying styles more selectively to improve performance.

src/style.css

Comment on lines +55 to +70

+                color: #00acdc;
+              }
+              .navbar.is-active {
+                color: #00acdc;
+              }
+              .navbar.is-active::after {
+                content: '';
+                position: absolute;
+                bottom: 0;
+                left: 0;
+                width: 100%;
+                height: 4px;
+                border-radius: 8px;
+                background: #00acdc;

mateacademy-ai-mentor Dec 17, 2024

The color #00acdc is used multiple times. The task requires using a CSS variable for the blue color. Consider defining a CSS variable for this color and using it throughout the stylesheet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet